feat: [3/4] partial storage map queries#1428
Conversation
drahnr
force-pushed
the
bernhard-partial-storage-map-queries
branch
2 times, most recently
from
c5f05b6 to
5391d24
Compare
…to bernhard-partial-storage-map-queries
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you. It seems like some of the changes from the latest review got reverted and some comments that were marked as resolved were not resolved. Other than that, I think we are pretty close.
Actually - ignore this review from me as I reviewed before realizing that there are still some merge conflicts with the base branch. Let's resolve the conflicts and then I can review again. |
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you. I left some more comments inline - once these are addressed, we should be good to merge.
Also, there is a small remaining merge conflict (I think the function just needs to be removed).
crates/proto/src/domain/account.rs
Outdated
There was a problem hiding this comment.
nit (and not from this PR): the "account storage map details" section is defined below (under line 471) - so, it is a bit odd to have this impl here rather than in that section.
3 participants
Migration of #1158
Targets #1394
Outstanding work:
Split #1178 into a serialization and usage piece, which can replace the current naiive serialization to a byte blob, but use a full PartialSmt protobuf representation
Scope
Implements the API to query partial storage maps based on an naiive approach (collecting SmtProofs, one per leaf).
Out of scope
Any optimization as outline by #1178 - this is follow-up material.